[Archive Migration] batch 2 of removing es_archives/empty_kibana #138208

LeeDr · 2022-08-05T18:02:06Z

Summary

Part of Meta issue: #102552

This PR removes the use of es_archives/empty_kibana and replaced it with calls to kibanaServer.savedObjects.cleanStandardList. Similar to this PR #138189

WARNING: I should note that there is a difference between
esArchiver.load('x-pack/test/functional/es_archives/empty_kibana'); (which I think deletes the entire .kibana index) and
kibanaServer.savedObjects.cleanStandardList(); (which can only delete savedObjects of types supported by the Saved Object API in the DEFAULT SPACE). There might be some types of saved objects which are not deleted. The full set we delete are here https://github.com/elastic/kibana/blob/main/packages/kbn-test/src/kbn_client/kbn_client_saved_objects.ts#L225 And saved objects in other spaces wouldn't be deleted. I mostly used find/replace to generate this PR and didn't review all the tests in detail. But the tests passed! The fact that our tests are broken up into so many separate configs probably helps with the isolation of tests.
I find that generally tests that created a space also already had an after method that deletes the space (and everything in it).

…StandardList

…-ref HEAD~1..HEAD --fix'

…remove_empty_kibana_2

LeeDr · 2022-08-05T20:35:50Z

There seem to be some tests here which are creating spaces and not cleaning them up at the end, which causes a failure when the next test tries to create a space which already exists. I don't know these tests but it seems like these should be matching pairs of setupSpacesAndUsers and tearDown? This could be a case where es_archives/empty_kibana would have wiped out everything in the .kibana index including spaces but cleanStandardList doesn't do that.

main/kibana - (remove_empty_kibana_2) > grep -r setupSpacesAndUsers x-pack/test/alerting_api_integration/security_and_spaces/group2/* | wc -l
      13
main/kibana - (remove_empty_kibana_2) > 
main/kibana - (remove_empty_kibana_2) > grep -r tearDown x-pack/test/alerting_api_integration/security_and_spaces/group2/* | wc -l           
       7

LeeDr · 2022-08-15T14:09:16Z

@elasticmachine merge upstream

kibanamachine · 2022-08-15T14:09:18Z

merge conflict between base and head

LeeDr · 2022-08-15T17:46:23Z

@elasticmachine merge upstream

elasticmachine · 2022-08-15T17:46:34Z

Pinging @elastic/fleet (Team:Fleet)

kpollich

Fleet changes

LeeDr · 2022-08-15T18:55:46Z

x-pack/test/common/services/spaces.ts

      const { data, status, statusText } = await axios.delete(`/api/spaces/space/${spaceId}`);

      if (status !== 204) {
-        throw new Error(
+        log.debug(


I just want to call this out for reviewers. The issue here is that some tests are still unloading an es_archive which contains spaces. This method was failing if trying to delete a space which has already been removed by unloading the es_archive. It appears that this is primarily used as a tearDown step and not any tests.

Yeah my eyebrow raised on this one at first, but I concur. Good call.

pmuellr

ResponseOps changes LGTM

pmuellr · 2022-08-15T21:44:04Z

Though the ResponseOps changes LGTM, I peeked at cleanStandardList() implementation, and we may have a problem with this:

kibana/packages/kbn-test/src/kbn_client/kbn_client_saved_objects.ts

Lines 223 to 230 in 81d7a6f

    
           public async cleanStandardList(options?: { space?: string }) { 
        
             // add types here 
        
             const types = [ 
        
               'url', 
        
               'index-pattern', 
        
               'action', 
        
               'query', 
        
               'alert',

You're not supposed to delete alerting rule SO's (type: alert) directly, as we have task SO's (in the task manager index) that are closely associated with the rule SO's. If you only delete the rule SO, but not the task manager SO, the rule will still attempt to run, as that determination is from the task manager SO, but the rule will fail since there is no rule SO.

While this is likely quite survivable today, since typically nothing bad happens except for excessive logging, we have seen some scenarios in the field of this affecting task manager health (it starts to complain about the failure rate if you delete a lot like this), so it would really be best NOT to delete the rule SO's, just by themselves. It will also cause these "zombie" tasks to delay execution of rules that are currently under test, generally making them run longer, or worse case fail with their own sorts of "timeouts".

I feel like perhaps we can have a way to do alternate delete processing for some of the types, so instead of deleting the rule SO via SO APIs, we'd delete it via the alerting APIs, which delete both SO docs.

LeeDr · 2022-08-15T22:32:38Z

I feel like perhaps we can have a way to do alternate delete processing for some of the types, so instead of deleting the rule SO via SO APIs, we'd delete it via the alerting APIs, which delete both SO docs.

If rules are defined within a space, does deleting the space take care of the task manager index SOs?

For rules not in a space, it sounds like we would have to use api/alerting/rules/_find and then DELETE api/alerting/rule/ for each id that was found. And it could be paginated... Is there already a cleanup method somewhere in the repo for that?

pheyos

ML changes LGTM

efegurkan

Enterprise Search changes look good.

tsullivan

LGTM

parkiino

onboarding lifecycle management changes lgtm

kibana-ci · 2022-08-23T19:31:40Z

💚 Build Succeeded

Buildkite Build
Commit: bc026ff

Metrics [docs]

✅ unchanged

History

💛 Build #65163 was flaky 4a38e01
💚 Build #65109 succeeded 4c85217
💔 Build #64972 failed 7733c90
💔 Build #64942 failed 5f3d706
💔 Build #64678 failed d51b4d5
💔 Build #64109 failed 4c2e16c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…stic#138208) * replace es_archives/empty_kibana with kibanaServer.savedObjects.cleanStandardList * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * add missing kibanaServer * add a tearDown * revert changes that don't pass * revert fleet_setup, delete spaces in tearDown * Don't fail on deleting spaces * revert file for failing test Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit f2c20b7)

kibanamachine · 2022-08-23T19:35:17Z

💚 All backports created successfully

Status	Branch	Result
✅	8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…8208) (#139318) * replace es_archives/empty_kibana with kibanaServer.savedObjects.cleanStandardList * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * add missing kibanaServer * add a tearDown * revert changes that don't pass * revert fleet_setup, delete spaces in tearDown * Don't fail on deleting spaces * revert file for failing test Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit f2c20b7) Co-authored-by: Lee Drengenberg <lee.drengenberg@elastic.co>

…stic#138208) * replace es_archives/empty_kibana with kibanaServer.savedObjects.cleanStandardList * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * add missing kibanaServer * add a tearDown * revert changes that don't pass * revert fleet_setup, delete spaces in tearDown * Don't fail on deleting spaces * revert file for failing test Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

replace es_archives/empty_kibana with kibanaServer.savedObjects.clean…

7bb68d7

…StandardList

LeeDr mentioned this pull request Aug 5, 2022

[meta] Migrate .kibana esArchives to kbnArchiver #102552

Closed

kibanamachine and others added 3 commits August 5, 2022 18:08

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

49bb523

…-ref HEAD~1..HEAD --fix'

add missing kibanaServer

9f49004

Merge branch 'remove_empty_kibana_2' of github.com:LeeDr/kibana into …

1ea277b

…remove_empty_kibana_2

LeeDr added test_xpack_functional release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Feature:Functional Testing labels Aug 5, 2022

Lee Drengenberg added 5 commits August 9, 2022 14:34

add a tearDown

45e9642

revert changes that don't pass

4c2e16c

revert fleet_setup, delete spaces in tearDown

d51b4d5

Don't fail on deleting spaces

5f3d706

revert file for failing test

7733c90

merge

4c85217

LeeDr marked this pull request as ready for review August 15, 2022 17:29

LeeDr requested review from a team as code owners August 15, 2022 17:29

LeeDr requested a review from a team August 15, 2022 17:29

LeeDr requested review from a team as code owners August 15, 2022 17:29

LeeDr requested review from joeypoon and parkiino August 15, 2022 17:29

Merge branch 'main' into remove_empty_kibana_2

4a38e01

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Aug 15, 2022

kpollich approved these changes Aug 15, 2022

View reviewed changes

LeeDr commented Aug 15, 2022

View reviewed changes

pmuellr approved these changes Aug 15, 2022

View reviewed changes

pheyos approved these changes Aug 16, 2022

View reviewed changes

wayneseymour approved these changes Aug 16, 2022

View reviewed changes

efegurkan approved these changes Aug 16, 2022

View reviewed changes

joeypoon approved these changes Aug 23, 2022

View reviewed changes

tsullivan approved these changes Aug 23, 2022

View reviewed changes

Merge branch 'main' into remove_empty_kibana_2

bc026ff

parkiino approved these changes Aug 23, 2022

View reviewed changes

LeeDr enabled auto-merge (squash) August 23, 2022 19:13

LeeDr merged commit f2c20b7 into elastic:main Aug 23, 2022

kibanamachine added the v8.5.0 label Aug 23, 2022

kibanamachine mentioned this pull request Aug 23, 2022

[8.4] [Archive Migration] batch 2 of removing es_archives/empty_kibana (#138208) #139318

Merged

kibanamachine added the v8.4.0 label Aug 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Archive Migration] batch 2 of removing es_archives/empty_kibana #138208

[Archive Migration] batch 2 of removing es_archives/empty_kibana #138208

LeeDr commented Aug 5, 2022 •

edited by kibanamachine

Loading

LeeDr commented Aug 5, 2022

LeeDr commented Aug 15, 2022

kibanamachine commented Aug 15, 2022

LeeDr commented Aug 15, 2022

elasticmachine commented Aug 15, 2022

kpollich left a comment

LeeDr Aug 15, 2022

wayneseymour Aug 16, 2022

pmuellr left a comment

pmuellr commented Aug 15, 2022

LeeDr commented Aug 15, 2022

pheyos left a comment

efegurkan left a comment

tsullivan left a comment

parkiino left a comment

kibana-ci commented Aug 23, 2022

kibanamachine commented Aug 23, 2022

[Archive Migration] batch 2 of removing es_archives/empty_kibana #138208

[Archive Migration] batch 2 of removing es_archives/empty_kibana #138208

Conversation

LeeDr commented Aug 5, 2022 • edited by kibanamachine Loading

Summary

LeeDr commented Aug 5, 2022

LeeDr commented Aug 15, 2022

kibanamachine commented Aug 15, 2022

LeeDr commented Aug 15, 2022

elasticmachine commented Aug 15, 2022

kpollich left a comment

Choose a reason for hiding this comment

LeeDr Aug 15, 2022

Choose a reason for hiding this comment

wayneseymour Aug 16, 2022

Choose a reason for hiding this comment

pmuellr left a comment

Choose a reason for hiding this comment

pmuellr commented Aug 15, 2022

LeeDr commented Aug 15, 2022

pheyos left a comment

Choose a reason for hiding this comment

efegurkan left a comment

Choose a reason for hiding this comment

tsullivan left a comment

Choose a reason for hiding this comment

parkiino left a comment

Choose a reason for hiding this comment

kibana-ci commented Aug 23, 2022

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Aug 23, 2022

💚 All backports created successfully

Questions ?

LeeDr commented Aug 5, 2022 •

edited by kibanamachine

Loading